Skip to content

Orb of Ankou 1.21.5 #1128

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 24 commits into
base: update-1.21.5
Choose a base branch
from
Open

Conversation

BPR02
Copy link
Member

@BPR02 BPR02 commented May 10, 2025

Fixes various pneumas (agile, anchoring, bashing, blasting, dashing, enduring, pricking, feigning, soaring).

Uses fall damage multiplier attribute for agile instead of a clock.

Uses player motion library (v1.4.2) to implement dashing mechanic.

Loot tables now exist for all soul shards.

Soul Shards and Orbs of Ankou are now non stackable (stack size of 1).

Upgrade paths are implemented by throwing the item into a soul forge and extinguishing it. The entity data for failed items is now based on loot tables instead of copying the exact item data from the soul forge.

@Bloo-dev
Copy link
Member

Does this include souls for the Armadillo, the Bogged, and the Breeze? Asking as they are not ticked off yet in #1010.

@BPR02
Copy link
Member Author

BPR02 commented May 11, 2025

Armadillo, Bogged, and Breeze souls were added in #1054

BPR02 added 9 commits May 13, 2025 12:54
- inline functions that aren't called from multiple places
- use random
- shard and essence entities grab loot table item instead of copying exact item (to act as an upgrade path)
- TODO: orb upgrade path using data from shard loot tables
@BPR02 BPR02 marked this pull request as ready for review May 15, 2025 00:00
@BPR02
Copy link
Member Author

BPR02 commented May 15, 2025

This uses the player motion library, which is a potential issue since it's the first time adding a non-gm4 library as a dependency. AFAIK there is no way for our dependency checker functions (now automatically generated based on beet.yaml) to account for external libraries and disable the module if an unsupported version of player motion is installed. At the very most, the dashing pneuma would change behavior, which isn't that big of a deal. Should this be addressed before merging this change?

@Bloo-dev
Copy link
Member

Bloo-dev commented May 16, 2025

I'm not too keen on relying on external data pack libraries

@misode
Copy link
Member

misode commented May 17, 2025

I don't think relying on external libraries is a bad thing, as long as we properly pin the version or copy it into our sources. But I'm more wondering why this library is suddenly necessary and wasn't in the previous version. Could we not update OoA to 1.21.5 without this new mechanic?

@BPR02
Copy link
Member Author

BPR02 commented May 17, 2025

The library isn't necessary, but it provides a much better UX and UI. The previous system locked the player into the trajectory, not allowing them to move away from the initial dash (since it makes the player ride an armor stand which is given motion) and it also used weird armor stand motion which broke. Instead of fixing the broken armor stand motion I decided it was better to do it the "proper" way with player motion.

I can try to fix dashing without player motion if we don't wanna deal with it for this update.

@misode
Copy link
Member

misode commented May 17, 2025

Using the player_motion library seems fine, as long as:

  1. We won't run into problems when multiple modules try to rely on it
  2. We make sure the build is still 100% reproducible
  3. We can handle updating the library to a new version
  4. We can handle when player_motion is included in a non-GM4 data pack, possibly a different version

- adds the player motion library as a namespaced and versioned GM4 library with proper call function tags
@BPR02
Copy link
Member Author

BPR02 commented May 18, 2025

Okay, this PR now adds player motion as a GM4 library (lib_player_motion) which has the following differences:

  • the targeted player motion library is locked to one specific version of the player motion library (v1.4.2 rn since it's the latest)
  • the library uses gm4_player_motion instead of player_motion namespaces
  • #minecraft:load has been deleted to allow for our lantern load and versioning plugin to handle load
  • Function tags #gm4_player_motion:launch_xyz and #gm4_player_motion:launch_looking are added to allow for version-checked function calls similar to our other libraries
  • The marker spawned at 0 0 0 (for trig) is spawned (and targeted) with a different UUID to prevent conflict with the normal library marker

This handles all 4 criteria specified in the last message.

Additionally, this PR now adds a new plugin which refactors external libraries to prefix namespaces with "gm4_" and deletes any #minecraft:load and #minecraft:tick function tags that are used.

Copy link
Member

@Bloo-dev Bloo-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some more discussion we have concluded that inclusion of the player motion library as-is is not possible due to

  • it forceloading 0 0
  • it not using lantern load

We should instead fork the library and make it use our forceloaded chunk and lantern load, as well as proper versioning. Alongside these changes, using a different UUID for the marker and using the gm4_player_motion namespace are required for this PR to proceed.

- basically a fork that is renamespaced, version-checked, and uses the GM4 forceloaded chunk
- licensing might not be correct in the release download
@BPR02
Copy link
Member Author

BPR02 commented May 19, 2025

The player motion library has been "forked" (copied over manually) to use lantern load and our forceloaded chunk. Full changes can be found in the README of lib_player_motion.

I'm not entirely sure if the licensing is done correctly. MIT doesn't require us to list the changes made (like GPL3 does), so I think we should be fine just putting the original MIT license file in the data/gm4_player_motion folder? But I don't think that would be carried onto the download zip since it only carries over the main LICENSE file from the library root folder. Or do we just add the copyright onto our license file?

@BPR02 BPR02 requested a review from Bloo-dev May 19, 2025 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants